-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(cranelift-codegen): remove OnceLock
#8489
base: main
Are you sure you want to change the base?
refactor(cranelift-codegen): remove OnceLock
#8489
Conversation
This removes the dependency on `std::sync::SpinLock` by lifting the state out of a static and into the `Callee` struct.
SpinLock
OnceLock
ping @elliottt (or other cranelift folks on this), I don't know enough about |
This could possibly result in a performance degradation in the compiler: it removes the caching of the @JonasKruckenberg, could you say more about the motivation here? And have you measured the performance impact? |
Yeah thats a concern I had too. I'll do some benchmarking next week and report back 👍 |
Ah, one other option could be to put it in the |
FWIW I ran this locally and on sightglass it reported a 1-3% slowdown in compiling spidermonkey.wasm, but no changes compiling other benchmarks. (I didn't re-run to see if that number was noise) |
@uweigand introduced the Regarding the main point of this PR though: I believe we can get rid of the need for |
It's not completely certain, depending on choice of frame pointer register, but in general it would be good to have this option. Allocatable registers in general may depend on calling convention. |
This removes the dependency on
std::sync::OnceLock
by lifting the state out of a static and into theCallee
struct.This PR also removes the
call_conv
parameter since it was not used by any of the trait implementations.